When creating a process, set the FD_CLOEXEC flag on the master PTY#193
Open
romainreignier wants to merge 1 commit intoxqms:masterfrom
Open
When creating a process, set the FD_CLOEXEC flag on the master PTY#193romainreignier wants to merge 1 commit intoxqms:masterfrom
romainreignier wants to merge 1 commit intoxqms:masterfrom
Conversation
To avoid a leak of the PTY file descriptor in the child process, the ROS Node. This allows to close the master fd in the child process after execvp() is executed. From man fcntl: > FD_CLOEXEC, the close-on-exec flag. If the FD_CLOEXEC bit is set, > the file descriptor will automatically be closed during a successful > execve(2). Before this, the child processes (the shim, then the ROS node) inherited all the `master` and `stderr_master` file descriptors of the other already-running nodes from the parent process. For example, with a launch file with 10 nodes, running `lsof -p $PID` with the pid of the nodes, there was a growing number of `/dev/ptmx` lines for the ROS nodes process. While when launched with roslaunch, there is node. Command used: `sudo lsof -p $PID | grep ptmx | wc -l` | Node number | Number of /dev/ptmx | |-------------|---------------------| | 1 | 0 | | 2 | 2 | | 3 | 4 | | 4 | 6 | | 10 | 18 | With this fix, the number is always zero.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To avoid a leak of the PTY file descriptor in the child process, the ROS Node.
This allows to close the master fd in the child process after execvp() is executed.
From man fcntl:
Before this, the child processes (the shim, then the ROS node) inherited all the
masterandstderr_masterfile descriptors of the other already-running nodes from the parent process.For example, with a launch file with 10 nodes, running
lsof -p $PIDwith the pid of the nodes, there was a growing number of/dev/ptmxlines for the ROS nodes process. While when launched with roslaunch, there is node.Command used:
sudo lsof -p $PID | grep ptmx | wc -lWith this fix, the number is always zero.